Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#944] Improve performance #2108

Merged

Conversation

SkyBlaise99
Copy link
Contributor

@SkyBlaise99 SkyBlaise99 commented Feb 6, 2024

Part of #944

Proposed commit message

Currently, the authorship credit analysis process exceeds an hour in
duration, which is significantly longer than the mere 5 minutes required
when the feature is deactivated.

Let's speed up the performance by implementing a caching mechanism and
refining the dynamic programming algorithm utilized for computing the
Levenshtein distance.

Other information

Statistics obtained by running on CS2103/T AY22/23 Semester 2 student repositories.

Forked here and some statistics recorded on the Github Actions.

Case Runtime
Base Case 1h 30min
Cache 15min
Lev Distance 1h 8min
Cache + Lev Distance 13 min

SkyBlaise99 and others added 30 commits September 12, 2023 21:45
Currently, users are unable to select a zoom range that includes 
the until date.

This results in misleading data being presented to users.
…ense#2041)

Chrome bug is causing cypress to fail to open a browser on Github 
Actions, causing frontend tests and CI to fail. Upgrading cypress 
to greater than 12.15.0 will fix this issue.

Let's upgrade cypress to fix the failing CI.
Currently, there is still some JavaScript code which remains 
unmigrated. This allows for type unsafe code to be written, 
potentially resulting in unintended behavior.

Let's migrate the rest of the JavaScript code to TypeScript 
code to facilitate future changes to the code.
…posense#2040)

Currently, there is still some JavaScript code which remains 
unmigrated. This allows for type unsafe code to be written, 
potentially resulting in unintended behavior.

Let's migrate the rest of the JavaScript code to TypeScript 
code to facilitate future changes to the code.
Currently, Cypress zoom feature tests are failing due to a recent change
in behavior caused by a bug fix. With the tests failing, we are unable
to detect any future regressions.

Let's update the Cypress tests to test for the new intended behavior.
…#2043)

Currently, there is still some JavaScript code which remains unmigrated.
This allows for type unsafe code to be written, potentially resulting in
unintended behavior.

Let's migrate random-color-generator.js JavaScript code to TypeScript
code to facilitate future changes to the code.
…sense#2036)

Currently, there is still some JavaScript code which remains unmigrated.
This allows for type unsafe code to be written, potentially resulting in
unintended behavior.

Let's migrate the rest of the JavaScript code to TypeScript code to
facilitate future changes to the code.
Currently, there is still some JavaScript code which remains unmigrated.
This allows for type unsafe code to be written, potentially resulting in
unintended behavior.

Let's migrate the rest of the JavaScript code to TypeScript code to
facilitate future changes to the code.
Bumps [zod](https://github.com/colinhacks/zod) from 3.20.6 to 3.22.3.
- [Release notes](https://github.com/colinhacks/zod/releases)
- [Changelog](https://github.com/colinhacks/zod/blob/master/CHANGELOG.md)
- [Commits](colinhacks/zod@v3.20.6...v3.22.3)

---
updated-dependencies:
- dependency-name: zod
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [@cypress/request](https://github.com/cypress-io/request) to 3.0.1 and updates ancestor dependency [cypress](https://github.com/cypress-io/cypress). These dependencies need to be updated together.


Updates `@cypress/request` from 2.88.12 to 3.0.1
- [Release notes](https://github.com/cypress-io/request/releases)
- [Changelog](https://github.com/cypress-io/request/blob/master/CHANGELOG.md)
- [Commits](cypress-io/request@v2.88.12...v3.0.1)

Updates `cypress` from 12.17.4 to 13.3.0
- [Release notes](https://github.com/cypress-io/cypress/releases)
- [Changelog](https://github.com/cypress-io/cypress/blob/develop/CHANGELOG.md)
- [Commits](cypress-io/cypress@v12.17.4...v13.3.0)

---
updated-dependencies:
- dependency-name: "@cypress/request"
  dependency-type: indirect
- dependency-name: cypress
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Currently, there is still some JavaScript code which remains unmigrated.
This allows for type unsafe code to be written, potentially resulting in
unintended behavior.

Let's migrate the rest of the JavaScript code to TypeScript code to
facilitate future changes to the code.
Currently, when granularity is set to day or week, clicking on a ramp
will open up a zoom view where commit messages are not being displayed
and sorting by insertions does not result in any sorting. 

Let's fix the unintended behaviour of the zoom view.
Currently, there is still some JavaScript code which remains unmigrated.
This allows for type unsafe code to be written, potentially resulting in
unintended behavior.

Let's migrate repo-sorter.js to TypeScript code to facilitate future
changes to the code.
Currently, there is still some JavaScript code which remains unmigrated.
This allows for type unsafe code to be written, potentially resulting in
unintended behavior.

Let's migrate safari_date.js to TypeScript code to facilitate future
changes to the code.
Currently, frontend linter is failing due to lint scripts 
checking javascript files, the last of which has been 
removed in PR reposense#2053.

Lets update the lint command to exclude javascript 
files front the check.
@SkyBlaise99 SkyBlaise99 changed the title 944 improve performance [#944] Improve Performance Feb 6, 2024
@SkyBlaise99 SkyBlaise99 changed the title [#944] Improve Performance [#944] Improve performance Feb 6, 2024
@SkyBlaise99
Copy link
Contributor Author

@damithc Hi prof, the part related to lev dist is here.

@damithc
Copy link
Collaborator

damithc commented Feb 6, 2024

@damithc Hi prof, the part related to lev dist is here.

@SkyBlaise99 Got it. The increase in complexity (in that method) is not that bad.

@SkyBlaise99 SkyBlaise99 marked this pull request as ready for review February 6, 2024 12:09
@ckcherry23 ckcherry23 requested a review from a team February 6, 2024 14:55
@ckcherry23 ckcherry23 requested a review from a team February 19, 2024 06:25
Copy link
Member

@MarcusTXK MarcusTXK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, code is well commented and clean. Great job with the improvement in the performance

@MarcusTXK MarcusTXK requested a review from a team March 2, 2024 05:47
Copy link
Contributor

@gok99 gok99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - haven't reasoned through the details of the algorithm updates, but I'll trust the comprehensiveness of the tests.

I'm curious what the memory consumption hit is like if you happened to measure this too - the intelliJ profiler should be able to provide some information.

@gok99 gok99 added s.ToMerge and removed s.ToReview labels Mar 2, 2024
@SkyBlaise99
Copy link
Contributor Author

SkyBlaise99 commented Mar 2, 2024

I'm curious what the memory consumption hit is like if you happened to measure this too - the intelliJ profiler should be able to provide some information.

I'm not too familiar with intellij profiler. I tried using it initially but due to the long runtime on local env, I stopped using it. Instead I used this telemetry which also measures the ram usage, but it shows a rough scale and not the exact number.

Currently I'm looking at 2GB vs 1.5GB, a 0.5GB increase in ram usage when caching is enabled. As compared to 7GB limit for github actions, this should still be okay.

@gok99 gok99 merged commit fdd2dae into reposense:944-analyze-authorship Mar 3, 2024
10 checks passed
Copy link
Contributor

github-actions bot commented Mar 3, 2024

The following links are for previewing this pull request:

@SkyBlaise99 SkyBlaise99 deleted the 944-improve-performance branch March 3, 2024 07:00
@SkyBlaise99 SkyBlaise99 restored the 944-improve-performance branch March 3, 2024 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants